Comment from gps in bug 1376803: "Also, if you write version control code, consider sticking it in python/mozversioncontrol. Not a blocker. But a nice to have so we don't reinvent as many wheels."

Hi Sylvestre, I would like to work on this.

sure, please give it a try!

gps is the pro for this, redirecting to him the review.

Comment on attachment 8897597 [details] Bug 1387035 - Update clang-format methods to use mozversioncontrol for hg/git detection https://reviewboard.mozilla.org/r/168872/#review174238 ::: tools/mach_commands.py:265 (Diff revision 1) > def run_clang_format_diff(self, clang_format_diff, show): > # Run clang-format on the diff > # Note that this will potentially miss a lot things > from subprocess import Popen, PIPE > > - if os.path.exists(".hg"): > + repository = mozversioncontrol.get_repository_object(self.topsrcdir) Use ``self.repository``. ::: tools/mach_commands.py:267 (Diff revision 1) > diff_process = Popen(["hg", "diff", "-U0", "-r", ".^", > "--include", "glob:**.c", "--include", "glob:**.cpp", > "--include", "glob:**.h", > "--exclude", "listfile:.clang-format-ignore"], stdout=PIPE) > else: > git_process = Popen(["git", "diff", "--no-color", "-U0", "HEAD^"], stdout=PIPE) > try: > diff_process = Popen(["filterdiff", "--include=*.h", Ideally these pieces would be abstracted and exposed as a method on the repository object. But that's a bit of scope bloat. The change as-is is better than it was before. ::: tools/mach_commands.py:340 (Diff revision 1) > > # Run clang-format > cf_process = Popen(args) > if show: > # show the diff > - if os.path.exists(".hg"): > + repository = mozversioncontrol.get_repository_object(self.topsrcdir) Use ``self.repository``.

(In reply to Gregory Szorc [:gps] from comment #5) > ::: tools/mach_commands.py:267 > (Diff revision 1) > > diff_process = Popen(["hg", "diff", "-U0", "-r", ".^", > > "--include", "glob:**.c", "--include", "glob:**.cpp", > > "--include", "glob:**.h", > > "--exclude", "listfile:.clang-format-ignore"], stdout=PIPE) > > else: > > git_process = Popen(["git", "diff", "--no-color", "-U0", "HEAD^"], stdout=PIPE) > > try: > > diff_process = Popen(["filterdiff", "--include=*.h", > > Ideally these pieces would be abstracted and exposed as a method on the > repository object. But that's a bit of scope bloat. The change as-is is > better than it was before. > I can also do that change, but I need some advise about the what the method signature should look like. All current methods of mozversioncontrol Repository class are returning full output string, never a PIPE, so I was thinking to introduce: def get_changes_to_format(self, pipe=False): """Return diffs of modified files that needs to be formatted. :param pipe: if True return a Popen pipe, else the output string. """ Are you ok with that ?

Comment on attachment 8897597 [details] Bug 1387035 - Update clang-format methods to use mozversioncontrol for hg/git detection https://reviewboard.mozilla.org/r/168872/#review174498 ::: tools/mach_commands.py:274 (Diff revision 1) > "--include", "glob:**.h", > "--exclude", "listfile:.clang-format-ignore"], stdout=PIPE) > else: > git_process = Popen(["git", "diff", "--no-color", "-U0", "HEAD^"], stdout=PIPE) > try: > diff_process = Popen(["filterdiff", "--include=*.h", This code has been removed in bug 1387036 please update your code! thanks

Comment on attachment 8897597 [details] Bug 1387035 - Update clang-format methods to use mozversioncontrol for hg/git detection https://reviewboard.mozilla.org/r/168872/#review174720

Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0828f347805f Update clang-format methods to use mozversioncontrol for hg/git detection r=gps

Bravo for your patch! thanks!